feat: add DRC check for vias too close to pads#113
feat: add DRC check for vias too close to pads#113lucaferri-dev wants to merge 8 commits intotscircuit:mainfrom
Conversation
Implements issue tscircuit#44 — a new DRC check that validates via placement relative to pads (both SMT pads and plated holes). Uses rect-to-circle and circle-to-circle distance calculations for accurate gap measurement. Closes tscircuit#44 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| import { expect, test, describe } from "bun:test" | ||
| import { checkViaToPadSpacing } from "lib/check-via-to-pad-spacing" | ||
| import type { AnyCircuitElement } from "circuit-json" | ||
|
|
||
| describe("checkViaToPadSpacing", () => { | ||
| test("returns error when via is too close to a rectangular SMT pad", () => { | ||
| const soup: AnyCircuitElement[] = [ | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "via1", | ||
| x: 0, | ||
| y: 0, | ||
| hole_diameter: 0.3, | ||
| outer_diameter: 0.6, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_smtpad", | ||
| pcb_smtpad_id: "pad1", | ||
| shape: "rect", | ||
| x: 0.5, | ||
| y: 0, | ||
| width: 0.4, | ||
| height: 0.3, | ||
| layer: "top", | ||
| }, | ||
| ] | ||
| // Via edge at 0.3, pad left edge at 0.3 => gap = 0mm, well below 0.2mm default | ||
| const errors = checkViaToPadSpacing(soup) | ||
| expect(errors).toHaveLength(1) | ||
| expect(errors[0].message).toContain("too close to pad") | ||
| }) | ||
|
|
||
| test("no error when via is far from pad", () => { | ||
| const soup: AnyCircuitElement[] = [ | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "via1", | ||
| x: 0, | ||
| y: 0, | ||
| hole_diameter: 0.3, | ||
| outer_diameter: 0.6, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_smtpad", | ||
| pcb_smtpad_id: "pad1", | ||
| shape: "rect", | ||
| x: 2, | ||
| y: 0, | ||
| width: 0.4, | ||
| height: 0.3, | ||
| layer: "top", | ||
| }, | ||
| ] | ||
| const errors = checkViaToPadSpacing(soup) | ||
| expect(errors).toHaveLength(0) | ||
| }) | ||
|
|
||
| test("returns error when via is too close to a circular SMT pad", () => { | ||
| const soup: AnyCircuitElement[] = [ | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "via1", | ||
| x: 0, | ||
| y: 0, | ||
| hole_diameter: 0.3, | ||
| outer_diameter: 0.6, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_smtpad", | ||
| pcb_smtpad_id: "pad1", | ||
| shape: "circle", | ||
| x: 0.7, | ||
| y: 0, | ||
| radius: 0.2, | ||
| layer: "top", | ||
| }, | ||
| ] | ||
| // center-to-center = 0.7, via radius = 0.3, pad radius = 0.2 => gap = 0.2mm | ||
| // gap + EPSILON (0.005) >= 0.2mm minSpacing => no error | ||
| const errors = checkViaToPadSpacing(soup) | ||
| expect(errors).toHaveLength(0) | ||
| }) | ||
|
|
||
| test("returns error when via is too close to a plated hole", () => { | ||
| const soup: AnyCircuitElement[] = [ | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "via1", | ||
| x: 0, | ||
| y: 0, | ||
| hole_diameter: 0.3, | ||
| outer_diameter: 0.6, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_plated_hole", | ||
| pcb_plated_hole_id: "hole1", | ||
| shape: "circle", | ||
| x: 0.6, | ||
| y: 0, | ||
| hole_diameter: 0.3, | ||
| outer_diameter: 0.5, | ||
| layers: ["top", "bottom"], | ||
| pcb_component_id: "comp1", | ||
| pcb_port_id: "port1", | ||
| }, | ||
| ] | ||
| // center-to-center = 0.6, via radius = 0.3, hole radius = 0.25 => gap = 0.05mm < 0.2mm | ||
| const errors = checkViaToPadSpacing(soup) | ||
| expect(errors).toHaveLength(1) | ||
| expect(errors[0].message).toContain("too close to pad") | ||
| }) | ||
|
|
||
| test("respects custom minSpacing parameter", () => { | ||
| const soup: AnyCircuitElement[] = [ | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "via1", | ||
| x: 0, | ||
| y: 0, | ||
| hole_diameter: 0.3, | ||
| outer_diameter: 0.6, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_smtpad", | ||
| pcb_smtpad_id: "pad1", | ||
| shape: "rect", | ||
| x: 1.0, | ||
| y: 0, | ||
| width: 0.4, | ||
| height: 0.3, | ||
| layer: "top", | ||
| }, | ||
| ] | ||
| // Via edge at 0.3, pad left edge at 0.8 => gap = 0.5mm | ||
| // With default 0.2mm: no error. With 0.6mm: error | ||
| expect(checkViaToPadSpacing(soup)).toHaveLength(0) | ||
| expect(checkViaToPadSpacing(soup, { minSpacing: 0.6 })).toHaveLength(1) | ||
| }) | ||
|
|
||
| test("returns empty array when no vias", () => { | ||
| const soup: AnyCircuitElement[] = [ | ||
| { | ||
| type: "pcb_smtpad", | ||
| pcb_smtpad_id: "pad1", | ||
| shape: "rect", | ||
| x: 0, | ||
| y: 0, | ||
| width: 0.4, | ||
| height: 0.3, | ||
| layer: "top", | ||
| }, | ||
| ] | ||
| expect(checkViaToPadSpacing(soup)).toHaveLength(0) | ||
| }) | ||
|
|
||
| test("returns empty array when no pads", () => { | ||
| const soup: AnyCircuitElement[] = [ | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "via1", | ||
| x: 0, | ||
| y: 0, | ||
| hole_diameter: 0.3, | ||
| outer_diameter: 0.6, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| ] | ||
| expect(checkViaToPadSpacing(soup)).toHaveLength(0) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
This test file violates the rule that states 'A *.test.ts file may have AT MOST one test(...), after that the user should split into multiple, numbered files. e.g. add1.test.ts, add2.test.ts'. The file contains 8 test() calls within a describe block (lines 6, 34, 60, 87, 117, 145, 161). To fix this violation, split the tests into multiple numbered files like check-via-to-pad-spacing1.test.ts, check-via-to-pad-spacing2.test.ts, etc., with each file containing only one test() call.
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
This file was from an earlier commit and has already been removed. The tests are now split into individual files under tests/lib/check-via-to-pad-spacing/ (one test per file).
One test per file per project convention. Moved tests from single file into check-via-to-pad-spacing/ directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
seveibar
left a comment
There was a problem hiding this comment.
no snapshots (not yet reviewable) please have one clear test with a pcb snapshot
Address review feedback: add SVG snapshots with error rendering to the error-detecting test cases so reviewers can visually verify the DRC check behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Added PCB snapshots with error rendering to the two error-detecting tests (via-too-close-to-rect-pad and via-close-to-plated-hole). Tests were already split into one-test-per-file in a previous commit. All 7 tests pass. |
|
Hi @seveibar, the PCB snapshots were actually pushed just after your review (commit 0334297). Both via-too-close-to-rect-pad and via-close-to-plated-hole tests now include SVG snapshots rendered with shouldDrawErrors: true. Each test file has exactly one test. Please take another look when you get a chance! |
lib/check-via-to-pad-spacing.ts
Outdated
| function getPadRadius(pad: Pad): number { | ||
| if (pad.type === "pcb_smtpad") { | ||
| if (pad.shape === "circle") return pad.radius | ||
| if ( | ||
| pad.shape === "rect" || | ||
| pad.shape === "rotated_rect" || | ||
| pad.shape === "pill" || | ||
| pad.shape === "rotated_pill" | ||
| ) { | ||
| return Math.max(pad.width, pad.height) / 2 | ||
| } | ||
| return 0 |
There was a problem hiding this comment.
Documentation bug: Comment on line 15-16 claims the function "returns the half-diagonal (conservative bounding circle)" for rectangular pads, but the implementation at line 27 returns Math.max(pad.width, pad.height) / 2, not the actual half-diagonal.
The correct half-diagonal would be:
return Math.sqrt(pad.width ** 2 + pad.height ** 2) / 2For a 1mm × 0.3mm pad:
- Current code: 0.5mm
- Correct diagonal: 0.523mm
This underestimates the bounding circle by ~4-5% for rectangular pads. While this fallback path is only used for unknown shapes (the main paths use proper rectangle-to-circle distance), it could still cause missed spacing violations for unsupported pad shapes.
| function getPadRadius(pad: Pad): number { | |
| if (pad.type === "pcb_smtpad") { | |
| if (pad.shape === "circle") return pad.radius | |
| if ( | |
| pad.shape === "rect" || | |
| pad.shape === "rotated_rect" || | |
| pad.shape === "pill" || | |
| pad.shape === "rotated_pill" | |
| ) { | |
| return Math.max(pad.width, pad.height) / 2 | |
| } | |
| return 0 | |
| function getPadRadius(pad: Pad): number { | |
| if (pad.type === "pcb_smtpad") { | |
| if (pad.shape === "circle") return pad.radius | |
| if ( | |
| pad.shape === "rect" || | |
| pad.shape === "rotated_rect" || | |
| pad.shape === "pill" || | |
| pad.shape === "rotated_pill" | |
| ) { | |
| return Math.sqrt(pad.width ** 2 + pad.height ** 2) / 2 | |
| } | |
| return 0 | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
The comment and implementation are now consistent — the current code on line 27 uses Math.sqrt(pad.width ** 2 + pad.height ** 2) / 2 which is the half-diagonal. This was fixed in commit 0c04968. The bot may have been looking at an older version.
- Add circuit JSON asset with board, components, traces and vias for a realistic PCB snapshot showing via-to-pad spacing violations - Fix getPadRadius to use actual half-diagonal (sqrt) instead of max/2 for the fallback bounding circle on rectangular pads Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lucaferri-dev
left a comment
There was a problem hiding this comment.
Added a realistic PCB snapshot test with a board, components (R1, C1), traces, and vias — one via placed too close to a pad. The snapshot clearly shows the violation highlighted in the SVG.
Also fixed the getPadRadius fallback to use the actual half-diagonal (Math.sqrt(w² + h²) / 2) instead of Math.max(w, h) / 2 as flagged in the Graphite review.
All 8 tests pass.
Renders SOIC8 chips with a via placed too close to a pad, showing the DRC error highlighted in the SVG snapshot. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lucaferri-dev
left a comment
There was a problem hiding this comment.
Added a clear PCB snapshot test (pcb-snapshot.test.tsx) that renders real SOIC8 chips using tscircuit, with a via placed too close to a pad. The SVG snapshot shows the DRC error highlighted on a realistic PCB layout.
Remove pcb-snapshot.test.tsx which produced an SVG with unrelated DRC errors (trace-to-edge, component-outside-board) that obscured the via-to-pad check. Keep via-too-close-to-pad-snapshot.test.ts which uses a clean JSON fixture and produces a focused snapshot showing only the relevant elements. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Snapshots have been added since your review (which was on commit 0783bc3, before any snapshots existed). Removed the noisy There are also snapshot tests for specific pad shapes (rect pad, plated hole). All 8 tests pass. |
seveibar
left a comment
There was a problem hiding this comment.
don't use the word soup, just one test is all that's needed.
You're not handling most smtpad/plated hole shapes, so this is going to create a lot of bugs. We need to have comprehensive coverage, you could use getBoundsOfPcbElements from circuit-json-util to approximate for now, and we can have a more intelligent computeGapBetweenCopper(elm1, elm2) inside circuit-json-util in the (near) future
Otherwise looks good
Use getBoundsOfPcbElements from circuit-json-util for generic pad shape handling instead of manually dispatching on every shape variant. Reduces to one clear test with a real tscircuit PCB snapshot per review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Addressed the review feedback:
All 67 tests pass. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lucaferri-dev
left a comment
There was a problem hiding this comment.
Addressed the feedback:
- Replaced manual shape-specific gap calculations with
getBoundsOfPcbElementsfrom circuit-json-util for comprehensive pad shape coverage - Consolidated all tests into a single test using the realistic circuit JSON fixture
All 68 tests pass.
Summary
Closes #44
Test plan